test: migrate pytest-operator to jubilant by yanksyoon · Pull Request #218 · canonical/github-runner-image-builder-operator

yanksyoon · 2026-04-16T03:10:44Z

Applicable spec:

Overview

Migrate pytest-operator to jubilant as a mitigation to bugs with websocket disconnects

Rationale

Issues with pytest-operator and juju leads to websocket disconnect errors, blocking CI

Juju Events Changes

Module Changes

Library Changes

Checklist

The charm style guide was applied
The contributing guide was applied
The changes are compliant with ISD054 - Managing Charm Complexity
The documentation for charmhub is updated.
The PR is tagged with appropriate label (urgent, trivial, senior-review-required, documentation)
The docs/changelog.md is updated with user-relevant changes in the format of keep a changelog v1.1.0
The application version is incremented in app/pyproject.toml

Only test changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…s_not_rebuild_image Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Migrates the repository’s integration test harness from pytest-operator/async Juju usage to the jubilant (synchronous) Juju wrapper to reduce CI failures from websocket disconnects.

Changes:

Replace pytest-operator + asyncio-based tests/fixtures with jubilant-based synchronous equivalents.
Rework integration fixtures to create/manage a temporary Juju model via jubilant.temp_model(), plus add --keep-models for debugging.
Update tox and integration requirements to drop async-only dependencies and add jubilant.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tox.ini	Replace pytest-operator/asyncio deps with jubilant in lint/integration envs.
tests/integration/types.py	Update test config typing from `Model` to `jubilant.Juju`.
tests/integration/test_upgrade.py	Port upgrade flow to `jubilant` APIs and synchronous waiting/log checks.
tests/integration/test_charm.py	Port integration tests to `jubilant` APIs and synchronous helpers/context management.
tests/integration/requirements.txt	Remove async-specific dependency (`nest_asyncio`).
tests/integration/helpers.py	Convert `wait_for`/`wait_for_images` from async to sync utilities.
tests/integration/conftest.py	Replace ops_test/model fixtures with a `jubilant`-managed temporary model + updated deploy/secret/config flows.
tests/conftest.py	Add `--keep-models` pytest CLI option to support jubilant temp model retention.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Use jubilant~=1.0 in lint env for consistent versioning with integration env - Wrap _change_cronjob_to_minutes yield in try/finally to ensure cron cleanup on test failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

florentianayuwono

lgtm thankyou!

encoding is only valid with filename, not handlers. The WatchedFileHandler is already created with encoding='utf-8'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yhaliaw

We need to review the logging of the jubilant first.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

tests/integration/conftest.py:410

This function uses await inside a non-async def and references app/app.model even though no app variable is defined (the deploy call doesn’t capture a return value). This is a syntax/runtime error; rewrite using jubilant operations (deploy, grant secrets, set config) without await, or make the whole fixture async and restore the asyncio toolchain.

    test_configs.juju.deploy(
        test_configs.charm_file,
        app_name,
        constraints=base_machine_constraint,
        config=app_config,
        log=False,
    )
    await app.model.grant_secret(openstack_password_secret.name, app.name)
    await app.model.grant_secret(script_secret.name, app.name)
    await app.set_config(
        {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Pin integration_test.yaml to operator-workflows@main instead of mutable feature branch ref - Convert openstack_password_secret_fixture from async pytest_asyncio to sync pytest fixture using juju.add_secret() - Convert app_config_fixture from async pytest_asyncio to sync pytest fixture (body was already synchronous) - Fix app_fixture return type (AsyncGenerator[Application] -> Generator[str]) and replace await calls with jubilant API: grant_secret(), config() - Fix _prepare_charmhub_app_config: remove async, replace ops_test param with juju: jubilant.Juju, fix NameError on test_configs - Fix app_on_charmhub_fixture: convert to sync pytest fixture, remove ops_test param, define missing app_name, replace all async/model calls with jubilant API, yield app name string instead of Application Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jubilant.Juju.deploy() does not accept a log parameter (it only exists on the private _cli method). Remove it from _deploy_test_charm and app_fixture to avoid TypeError at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yanksyoon

Initial review done

…alls - Flatten parenthesized tuple assignment for _prepare_charmhub_app_config to match black's preferred style - Remove log=False from juju.deploy() in test_cos_agent_relation; jubilant's deploy() does not accept a log parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…hub_fixture Was annotated as str instead of dict, causing mypy to error: Argument 'constraints' to 'deploy' of 'Juju' has incompatible type 'str'; expected 'Mapping[str, str] | None' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ndling

- Replace 'via' with 'through' or 'using' (Canonical.025a) - Replace 'etc' with 'and so on' (Canonical.025a) - Fix 'snapstore' -> 'the Snap Store' in changelog (Canonical.000) - Fix 'github actions' -> 'GitHub Actions' in architecture doc (Canonical.000) - Fix Ubuntu version format to include LTS suffix (Canonical.003) - Add 'aproxy' and 'opentelemetry' to vale accept list (product names) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… migration - Add update_status() to image.Observer so block_if_invalid_config works - Fix test_block_on_image_relation_not_ready to mock BuilderConfig.from_charm - Add tests for ImageConfig.from_charm, CloudConfig.from_charm, BuilderConfig.from_charm, upload_cloud_ids property, _get_num_parallel_build - Add tests for _parse_openstack_clouds_config with upload auth configs - Add tests for _parse_openstack_clouds_auth_configs_from_relation with units (complete and incomplete data) - Add test for _parse_script_secrets with no secrets configured - Add test for _on_image_relation_joined with no upload_cloud_ids Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Inline MagicMock call in test_charm.py to satisfy black formatting - Change B106 nosec to B105 for dict password string literal in test_state.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When upgrading from a charmhub edge revision that predates the openstack-password-secret config option, the upgrade test was failing because the old charm had no credentials and the new charm (which requires openstack-password-secret) also couldn't find them. Restore the fallback in _prepare_charmhub_app_config to set the legacy openstack-password config when the charmhub version doesn't yet expose openstack-password-secret. The constant is kept local to the test code since state.py no longer needs the legacy option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This reverts commit 473ff2e.

…pgrade test The app_fixture in test_upgrade.py was calling juju.refresh() without granting or setting the openstack-password-secret config. When upgrading from an older charmhub revision that didn't expose this config option, the refreshed charm would raise: InvalidCloudConfigError: Please supply OpenStack password via openstack-password-secret. Fix by adding openstack_password_secret to app_fixture's parameters and explicitly granting the secret and setting the config immediately after juju.refresh(), ensuring the new charm can read the OpenStack password. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- test_upgrade.py: add sudo to cat /var/log/juju/unit-*.log since the ubuntu user doesn't have read permission on juju log files; without it juju.ssh() raises CLIError on every poll, propagating immediately out of wait_for() - test_charm.py: wrap logrotate --debug command in sudo bash -c so that the 2>&1 redirect is interpreted by a shell, ensuring stderr (where logrotate outputs debug info) is merged into stdout and captured by juju.ssh() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

test: migrate pytest-operator to jubilant

6a3c8d3

github-actions Bot added the Libraries: Out of sync label Apr 16, 2026

yanksyoon and others added 2 commits April 16, 2026 11:36

Remove unused functools import from test_upgrade.py

2fac78e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Disable too-many-locals pylint warning for test_charm_another_app_doe…

65bca40

…s_not_rebuild_image Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yanksyoon changed the title ~~[WIP] test: migrate pytest-operator to jubilant~~ test: migrate pytest-operator to jubilant Apr 16, 2026

yanksyoon marked this pull request as ready for review April 16, 2026 03:43

yanksyoon requested review from cbartz, javierdelapuente and yhaliaw as code owners April 16, 2026 03:43

florentianayuwono requested a review from Copilot April 16, 2026 04:10

Copilot started reviewing on behalf of florentianayuwono April 16, 2026 04:10 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread tox.ini Outdated

Comment thread tests/integration/test_charm.py Outdated

Address PR review comments

a53df9b

- Use jubilant~=1.0 in lint env for consistent versioning with integration env - Wrap _change_cronjob_to_minutes yield in try/finally to ensure cron cleanup on test failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

florentianayuwono approved these changes Apr 16, 2026

View reviewed changes

yanksyoon and others added 2 commits April 16, 2026 13:35

Remove invalid encoding arg from basicConfig when using handlers

5962c15

encoding is only valid with filename, not handlers. The WatchedFileHandler is already created with encoding='utf-8'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

test: use os-agnostic workflow

db4e2d3

yhaliaw requested changes Apr 16, 2026

View reviewed changes

chore: disable deploy logs

953578e

yanksyoon requested a review from yhaliaw April 16, 2026 10:18

Merge branch 'main' into chore/migrate-to-jubilant

b461eb4

yanksyoon requested a review from Copilot April 21, 2026 01:19

Copilot started reviewing on behalf of yanksyoon April 21, 2026 01:19 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread tests/integration/conftest.py

Comment thread .github/workflows/integration_test.yaml Outdated

Comment thread tests/integration/conftest.py Outdated

Comment thread tests/integration/conftest.py

Comment thread tests/integration/conftest.py

yanksyoon and others added 2 commits April 21, 2026 12:29

yanksyoon commented Apr 21, 2026

View reviewed changes

yanksyoon and others added 2 commits April 22, 2026 00:48

florentianayuwono reviewed Apr 22, 2026

View reviewed changes

Comment thread tests/integration/conftest.py

ci: trigger ci

c0a2322

yanksyoon and others added 5 commits April 22, 2026 14:35

refactor: migrate to openstack-password-secret for secure password ha…

7fefe62

…ndling

test: use fix/pip-break-flag-workarounds integration test

659d6ac

ci: trigger ci

f17980f

revert: ci use main

b30b969

cbartz approved these changes Apr 22, 2026

View reviewed changes

yanksyoon and others added 2 commits April 22, 2026 15:55

fix: correct black formatting and bandit nosec tags in tests

9763100

- Inline MagicMock call in test_charm.py to satisfy black formatting - Change B106 nosec to B105 for dict password string literal in test_state.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

yhaliaw approved these changes Apr 23, 2026

View reviewed changes

yanksyoon and others added 21 commits April 23, 2026 10:03

Revert "fix: restore openstack-password fallback for upgrade test"

ef9bffd

This reverts commit 473ff2e.

ci: debug

23ea8e5

ci: add debug logging

648d6d8

ci: longer tmate

016585a

fix: pass os environ to subprocess

9911bc2

test: use juju 4.0

badcd02

chore: remove os environ passing

5ec9484

fix: ssh issues

2fb6cff

fix: ssh dir

ea57902

test: remove juju add key

d368889

test: juju ssh key fixture

23680bc

test: use juju ssh key fixture

d09648e

test: verify ssh key added

a1d3dca

fix: setup proxy environment on relation joined hook

cfc82e9

ci: trigger ci

29060e5

test: eject juju ssh key modification

5422cd8

test: applications keep models

13cfb72

test: add host ssh key

ec50c0e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: migrate pytest-operator to jubilant#218

test: migrate pytest-operator to jubilant#218
yanksyoon wants to merge 43 commits intomainfrom
chore/migrate-to-jubilant

yanksyoon commented Apr 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

florentianayuwono left a comment

Uh oh!

yhaliaw left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanksyoon left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

yanksyoon commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Rationale

Juju Events Changes

Module Changes

Library Changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

florentianayuwono left a comment

Choose a reason for hiding this comment

Uh oh!

yhaliaw left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanksyoon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yanksyoon commented Apr 16, 2026 •

edited

Loading